-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
Prevent the root path "/" from being removed #5899
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Prevent the root path "/" from being removed #5899
Conversation
WalkthroughThe trailing-slash removal logic for layout-type route nodes is refined to preserve the root path. The condition now excludes removing trailing slashes when the cleaned path is '/'. The pathless_layout case handling remains unaffected. Changes
Estimated code review effort🎯 1 (Trivial) | ⏱️ ~3 minutes
Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
Tip 📝 Customizable high-level summaries are now available in beta!You can now customize how CodeRabbit generates the high-level summary in your pull requests — including its content, structure, tone, and formatting.
Example instruction:
Note: This feature is currently in beta for Pro-tier users, and pricing will be announced later. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
8c6dcea to
07cf53c
Compare
|
View your CI Pipeline Execution ↗ for commit f28edc8
☁️ Nx Cloud last updated this comment at |
|
@cxa there's a failing unit test that needs some attention. |
|
first of all we need to understand why this fix is needed. when would this occur? |
07cf53c to
e6bf332
Compare
Without this fix, file route like |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
e2e/solid-start/basic-cloudflare/worker-configuration.d.ts (1)
10-14: StringifyValues change is sound; note behavior for union/optional envsThe new
StringifyValuespreserves literal string types and string unions while coercing non‑string values tostring, which matches env semantics. Just be aware that fields typed as unions including non‑string members (e.g.string | undefinedornumber | string) will still be widened tostring, so this only narrows fully string-typed bindings. That’s fine for this test, but worth keeping in mind if you later add richer Env typings.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
e2e/solid-start/basic-cloudflare/worker-configuration.d.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/src/routes/non-nested/named/$baz_.bar.tsx:3-5
Timestamp: 2025-09-22T00:56:49.237Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments (e.g., `$baz_` becomes `baz` in generated types) but should be preserved in base path segments. This is the correct behavior as of the fix in PR #5182.
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: packages/router-core/src/router.ts:2231-2245
Timestamp: 2025-10-01T18:30:26.591Z
Learning: In `packages/router-core/src/router.ts`, the `resolveRedirect` method intentionally strips the router's origin from redirect URLs when they match (e.g., `https://foo.com/bar` → `/bar` for same-origin redirects) while preserving the full URL for cross-origin redirects. This logic should not be removed or simplified to use `location.publicHref` directly.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5402
File: packages/router-generator/tests/generator/no-formatted-route-tree/routeTree.nonnested.snapshot.ts:19-21
Timestamp: 2025-10-08T08:11:47.088Z
Learning: Test snapshot files in the router-generator tests directory (e.g., files matching the pattern `packages/router-generator/tests/generator/**/routeTree*.snapshot.ts` or `routeTree*.snapshot.js`) should not be modified or have issues flagged, as they are fixtures used to verify the generator's output and are intentionally preserved as-is.
Learnt from: nlynzaad
Repo: TanStack/router PR: 5182
File: e2e/react-router/basic-file-based/tests/non-nested-paths.spec.ts:167-172
Timestamp: 2025-09-22T00:56:53.426Z
Learning: In TanStack Router, underscores are intentionally stripped from route segments during path parsing, but preserved in base path segments. This is the expected behavior implemented in PR #5182.
Learnt from: schiller-manuel
Repo: TanStack/router PR: 5330
File: e2e/react-start/custom-basepath/src/routeTree.gen.ts:58-61
Timestamp: 2025-10-01T18:31:35.420Z
Learning: Do not review files named `routeTree.gen.ts` in TanStack Router repositories, as these are autogenerated files that should not be manually modified.
🔇 Additional comments (1)
e2e/solid-start/basic-cloudflare/worker-configuration.d.ts (1)
5-7: Env ⇄ ProcessEnv wiring forMY_VARlooks correctTyping
Cloudflare.Env.MY_VARas the literal'Hello from Cloudflare'and threading it throughNodeJS.ProcessEnvviaStringifyValues<Pick<Cloudflare.Env, 'MY_VAR'>>will correctly narrowprocess.env.MY_VARin this fixture. No issues from a types or merging perspective.Also applies to: 16-18
f28edc8 to
6bc7f5b
Compare
This PR prevents the root path "/" from being emptied by
removeTrailingSlash.Summary by CodeRabbit
Release Notes
✏️ Tip: You can customize this high-level summary in your review settings.